Skip to content

Replace minimist with dashdash for CLI argument parsing#614

Closed
KAMRONBEK wants to merge 1 commit into
ds300:masterfrom
KAMRONBEK:fix-428-dashdash
Closed

Replace minimist with dashdash for CLI argument parsing#614
KAMRONBEK wants to merge 1 commit into
ds300:masterfrom
KAMRONBEK:fix-428-dashdash

Conversation

@KAMRONBEK

@KAMRONBEK KAMRONBEK commented Jul 17, 2026

Copy link
Copy Markdown

Closes #428.

What & why

minimist is deprecated and has a history of parsing bugs (see the issues
referenced in #428, plus #412 and the #250 thread). This PR ports
src/index.ts from minimist to dashdash,
a strict, spec-driven option parser (already a transitive dependency of this
project via other packages, so it adds no new install footprint).

Runtime behavior is kept identical for every documented flag and for
positional package-name arguments. The one intentional improvement is that
dashdash rejects unknown options with a clear message and exit code 1,
whereas minimist silently swallowed them (and could even eat the following
positional as the unknown flag's value — one of the "argument parser is broken"
symptoms #428 is about).

Relationship to #521 (minimist CVE bump)

#521 bumps minimist ^1.2.6 → ^1.2.8 (and @types/minimist ^1.2.2 → ^1.2.5)
to remediate CVE-2021-44906 (prototype pollution).

This PR obviates #521 for patch-package's own dependency: it removes
minimist and @types/minimist as direct dependencies entirely, so the
vulnerable direct edge #521 patches no longer exists. minimist still appears
in yarn.lock only as a transitive dependency of other packages
(minimist@^1.1.1, minimist@^1.2.0 — pulled in by e.g. mkdirp), which is
outside both this PR's and #521's scope. If maintainers prefer the minimal
CVE-only change, #521 is the safer pick; if they want the deprecated parser gone,
this PR supersedes it.

Flag-by-flag mapping

dashdash differs from minimist in two mechanical ways that the port accounts
for: (1) option names have their dashes converted to underscores in the parsed
result (--patch-dir → opts.patch_dir), and (2) positional arguments live in
opts._args (not opts._). Absent options are omitted from the result object,
so the existing "rebase" in argv / "append" in argv presence checks keep
working unchanged.

CLI flag Type minimist read dashdash read Notes
--help, -h bool argv.help || argv.h argv.help aliased via names: ["help","h"]; -h now sets help directly
--version, -v bool argv.version || argv.v argv.version aliased via names: ["version","v"]
--use-yarn bool argv["use-yarn"] argv.use_yarn
--case-sensitive-path-filtering bool argv["case-sensitive-path-filtering"] argv.case_sensitive_path_filtering
--reverse bool argv["reverse"] argv.reverse
--error-on-fail bool argv["error-on-fail"] argv.error_on_fail
--error-on-warn bool argv["error-on-warn"] argv.error_on_warn
--create-issue bool argv["create-issue"] argv.create_issue
--partial bool argv.partial argv.partial
--patch-dir <dir> string argv["patch-dir"] argv.patch_dir
--append [<name>] string argv.append, "append" in argv argv.append, "append" in argv presence check preserved
--rebase <patch> string argv.rebase, "rebase" in argv argv.rebase, "rebase" in argv presence check preserved
--include <regexp> string argv.include argv.include single value (see below)
--exclude <regexp> string argv.exclude argv.exclude single value (see below)
(positionals) package names — argv._ argv._args

Deliberate decisions / behavioral notes

  • --include / --exclude are type: "string" (single value), not
    arrayOfString.     The downstream consumer makeRegExp(reString: string, …)
    takes a string. Under minimist, repeating --include a --include b
    produced an array ["a","b"] that was then coerced to the string "a,b" and
    fed to new RegExp() — effectively undefined/broken behavior. dashdash with
    type: "string" gives last-value-wins ("b"), which is the sane single-value
    contract the code already assumes. No integration test passes the flag more
    than once.
  • Unknown options now error (exit 1 with a red message) instead of being
    silently absorbed. This is the desired fix, not a regression.
  • Parse errors are caught and printed via chalk.red(...) followed by
    process.exit(1), so users get a clean one-line message rather than a stack
    trace.
  • Bare --append / --rebase (no value) will now be a parse error rather than
    falling through to the graceful "you must specify…" message. This path is not
    reachable in any integration test (all usages supply a value, e.g.
    --rebase 0, --append 'WhileOne'), and requiring a value for a
    value-bearing flag is correct behavior.

Dependency changes (package.json)

  • Removed minimist (dependencies) and @types/minimist (devDependencies).
  • Added dashdash@^1.14.1 (dependencies) and @types/dashdash@^1.14.3
    (devDependencies). dashdash ships no bundled types, so the DefinitelyTyped
    package is used.

Verification (all run locally)

  • yarn build (tsc --project tsconfig.build.json) — passes, zero errors.
  • tslint --project tsconfig.json src/index.ts — clean.
  • ./run-tests.sh --runInBand (clean build + pack tarball + full Jest suite,
    incl. all integration + property-based tests) —
    47 suites passed, 660 tests passed, 224 skipped, 108 snapshots passed, 0 failed.
    Notable flag-exercising suites that passed: rebase-zero (--rebase/--append),
    include-exclude-regex-relativity & include-exclude-paths
    (--include/--exclude), error-on-warn, error-on-fail, partial-apply
    (--partial), reverse-option/reverse-multiple-patches (--reverse),
    custom-patch-dir (--patch-dir), create-issue (--create-issue),
    append-patches, happy-path-npm, happy-path-yarn.
  • Manual CLI exercise of the compiled dist/index.js:
    • --version / -v → prints patch-package <version> and exits (noop). ✔
    • --help → prints full usage. ✔
    • apply path (patch-package, --patch-dir mypatches, --reverse) →
      Applying patches… / No patch files found. ✔
    • --frobnicate somepkg (unknown flag) → unknown option: "--frobnicate",
      exit code 1. ✔
  • Parity harness: an offline diff of the downstream reads (patchDir,
    positionals, rebase/append presence, include/exclude, all booleans) across
    16 representative arg combinations showed identical results between the old
    minimist config and the new dashdash parser.

@KAMRONBEK
Replace minimist with dashdash for CLI arg parsing (ds300#428)
5ec945a 
minimist is deprecated and has a history of parsing bugs. Port the
argument parsing in src/index.ts to dashdash (already a transitive
dependency), keeping runtime behavior identical for every flag and for
positional package names.

dashdash converts dashes in option names to underscores in the parsed
result (--patch-dir -> opts.patch_dir) and puts positionals in
opts._args; 'help'/'h' and 'version'/'v' are declared as aliases. Absent
options are omitted from the result, so the existing "rebase"/"append"
in argv presence checks are unchanged. Unknown options are now rejected
with a clear message and exit 1 instead of being silently swallowed; the
parse call is wrapped so errors print cleanly.

Removes minimist and @types/minimist as direct deps (obviating the CVE
bump in ds300#521 for the direct dependency edge) and adds dashdash +
@types/dashdash.

Verified: yarn build (zero errors), tslint clean, and the full test
suite (./run-tests.sh --runInBand) passes 47 suites / 660 tests / 108
snapshots.
@KAMRONBEK KAMRONBEK closed this Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix argument parser: replace minimist with dashdash

1 participant

@KAMRONBEK